Skip to content

coverity Fixes #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
padelsbach merged 1 commit into from
Oct 3, 2025
Merged

coverity Fixes #293

padelsbach merged 1 commit into from
Oct 3, 2025

Conversation

@JeremiahM37
Copy link
Contributor

@JeremiahM37 JeremiahM37 commented Sep 26, 2025 •
edited
Loading

https://scan7.scan.coverity.com/#/project-view/55019/16638

This patch should resolve most of the Coverity defects I got on the first scan (ignoring test files and at least 3 false positive). This was not using --enable-all, so there will likely be more issues. Some of these fixes may be pointless and should have been marked as a false positive, but they seemed like actual issues upon first glance.

List of Coverity issues:

CID,Type,Impact,First Detected,Owner,Classification,Severity,Action,Component,Category
1658851,Logically dead code,Medium,09/22/25,Unassigned,Pending,Unspecified,Undecided,Other,Control flow issues
1658846,Logically dead code,Medium,09/22/25,Unassigned,Pending,Minor,Undecided,Other,Control flow issues
1658830,Explicit null dereferenced,Medium,09/22/25,Unassigned,Pending,Moderate,Undecided,Other,Null pointer dereferences
1658839,Use after free,High,09/22/25,Unassigned,Bug,Moderate,Undecided,Other,Memory - illegal accesses
1658838,Copy-paste error,Medium,09/22/25,Unassigned,Bug,Moderate,Undecided,Other,Incorrect expression
1658848,Use after free,High,09/22/25,Unassigned,Bug,Moderate,Undecided,Other,Memory - illegal accesses
1658836,Use after free,High,09/22/25,Unassigned,Bug,Moderate,Undecided,Other,Memory - illegal accesses
1658847,Use after free,High,09/22/25,Unassigned,Bug,Moderate,Undecided,Other,Memory - illegal accesses
1658831,Dereference after null check,Medium,09/22/25,Unassigned,Pending,Minor,Undecided,Other,Null pointer dereferences
1658840,Explicit null dereferenced,Medium,09/22/25,Unassigned,Pending,Unspecified,Undecided,Other,Null pointer dereferences
1658842,Dereference after null check,Medium,09/22/25,Unassigned,Pending,Unspecified,Undecided,Other,Null pointer dereferences
1658835,Resource leak,High,09/22/25,Unassigned,Bug,Moderate,Undecided,Other,Resource leaks
1658834,Explicit null dereferenced,Medium,09/22/25,Unassigned,Pending,Moderate,Undecided,Other,Null pointer dereferences
1658845,Dereference after null check,Medium,09/22/25,Unassigned,Pending,Moderate,Undecided,Other,Null pointer dereferences

  • I removed code in wp_aes_block.c and wp_des.c because I'm fairly certain its logically dead code since the condition can never be true. ( oLen is set to AES_BLOCK_SIZE and outSize is guaranteed to be at least that size)

  • wp _ecdh_exch.c, wp_ecx_exch.c, and wp_kdf_exch.c: set dst = NULL after OPENSSL_FREE(dst) to prevent use after free.

  • wp_mac_kmgmt.c: added ok && condition check to prevent operations when previous steps failed and data != null to prevent null pointer dereference.

-wp_dh_kmgmt.c
&dh->key.g was supposed to be &dh->key.p (wrong DH parameter)

  • wp_mac_sig.c: proper cleanup of error paths

Summary

  • Fixed 12 Coverity issues across 9 source files

@JeremiahM37 JeremiahM37 force-pushed the coverityFixes branch 4 times, most recently from 9b7f2cf to 01ebfa2 Compare September 29, 2025 20:03
@JeremiahM37 JeremiahM37 marked this pull request as ready for review October 1, 2025 16:01
padelsbach
padelsbach approved these changes Oct 2, 2025
View reviewed changes
Copy link
Contributor

@padelsbach padelsbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff!

@padelsbach padelsbach merged commit 93430f0 into wolfSSL:master Oct 3, 2025
165 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@padelsbach padelsbach padelsbach approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JeremiahM37 @padelsbach